feat(copilot): add GitHub Copilot as a first-class host platform - #22
Conversation
…d scripts - Updated asset providers to include "copilot" in agent customization. - Modified CLI options to support "copilot" as a valid platform. - Enhanced contract definitions to recognize "copilot" as a provider. - Updated report quality checks to include Copilot in session scope. - Adjusted report run and task loop scripts to accommodate Copilot. - Implemented new session analysis logic for Copilot, including event normalization and workspace handling. - Added Copilot-specific session analyzer and event processing in session analysis. - Updated help documentation and usage summaries to reflect Copilot integration. - Adjusted tests to validate changes related to Copilot support.
Adds the Copilot host support spec, a dedicated Copilot asset-route reference, and README/architecture/glossary coverage for the two new adapters. Corrects agent-facing provider enumerations that had silently omitted Qwen and Copilot: the agent-customize inventory CLI flags, the global-assets scope and home-directory list, the routing index entry, the ADR host-adapter comment, and the session-evidence --provider enum. Without these an agent reading the routing docs could never discover --provider copilot. Test assertions that pin docs prose (adapter table, plugin metadata roots, supported-platform list, report routing) are updated in lockstep. Spec: docs/specs/2026-07-29-copilot-host-support.md Verified: npm test 872/872, npm run pack:verify PASS (npm 314 / runtime zip 338), doc-link graph 34 files / 50 links with zero drift. Co-authored-by: Copilot App (Claude Opus 5) <223556219+Copilot@users.noreply.github.com>
phodal
left a comment
There was a problem hiding this comment.
Blocking on evidence correctness found with native GitHub Copilot CLI 1.0.74 data. The cross-platform CI suite and deterministic fixtures pass, and isolated marketplace installation works, but the public home override is ignored and real session usage, permission lifecycle, and coverage fields are misreported. Please address the four inline findings and add CLI/native-shape regression coverage before merge.
| "Collect configured agent-customize inventory for one provider as JSON.", | ||
| "Provider home overrides: --cursor-home, --qoder-home, --codex-home, --claude-home,", | ||
| "--qwen-home, --claude-state, --codex-app-path, --qoder-shared-client-cache-root.", | ||
| "--qwen-home, --copilot-home, --claude-state, --codex-app-path, --qoder-shared-client-cache-root.", |
There was a problem hiding this comment.
[P1] Forward --copilot-home into the collector. The help now advertises this flag, but the options object passed to collectAgentCustomizeInventory never includes copilotHome. In a local repro, requesting an isolated temp home still resolved /Users/phodal/.copilot and scanned the real user's assets. That breaks AC-6 and can cross the caller's intended authorization boundary. Please pass copilotHome: options["copilot-home"] and add a CLI-level regression test that asserts the resolved home and inventory both come from the override.
| }; | ||
| if (text) event.userVisibleAssistantMessage = true; | ||
| if (options.includeContent && text) event.content = text; | ||
| if (data.model) event.model = data.model; |
There was a problem hiding this comment.
[P1] Preserve the per-message output usage that Copilot actually records. In native 1.0.74 transcripts, 498 of 608 matching assistant messages carried numeric data.outputTokens (365,290 total), but this adapter keeps only model and emits type: "assistant". session-efficiency.mjs excludes ordinary assistant events from model responses, so the same data produces responseCount: 0 and tokenTotals: null. Please model these as partial response usage (without inventing input tokens or exact cost), retain a response/request identity for deduplication, and update AC-5/docs/tests to describe partial rather than wholly unobserved per-response usage.
There was a problem hiding this comment.
Rechecked at 58b93fd with the same native Copilot data. The companion events do recover 498 responses and 365,290 outputTokens, but the public usage-summary now emits {"inputTokens":0,"outputTokens":365290,"cacheReadInputTokens":0,"cacheCreationInputTokens":0}. Those zeroes come from the generic aggregation/serialization path, so CHS-AC-5’s “omit input/cache tokens rather than reporting them as zero” is still not true at the user-facing contract. Please preserve per-field observation through aggregation (for example, omit unobserved fields or expose an explicit observed-field mask without serializing missing fields as measured zero) and add a usage-summary regression that asserts only output usage is observed and exact cost remains unavailable.
| return events; | ||
| } | ||
|
|
||
| if (rawType === "session.permissions_changed" || rawType === "session.mode_changed") { |
There was a problem hiding this comment.
[P2] Normalize Copilot's actual permission lifecycle event names. The local transcripts contain 65 permission.requested and 65 permission.completed records; they do not use session.permissions_changed. Both currently fall through to metadata.*, so permission approvals/denials cannot contribute bounded operation-control or friction evidence even though the PR describes permission lifecycle support. Please map the request/result pair without retaining prompt or command payloads and cover the observed event shape in tests.
| transcript: { | ||
| sourceAvailable: Boolean(root?.exists), | ||
| workspaceSessions, | ||
| withTranscript: withTranscript.length, |
There was a problem hiding this comment.
[P2] Align this coverage payload with the public session-core-facts contract. This provider emits withTranscript, withoutTranscript, and emptyTranscripts, while safeSourceCoverage accepts withConversation, withRequest, unreadable, and outOfWindowSessions. In a native run with 21 matching workspace sessions, 16 transcripts, and 5 transcript-less sessions, the public facts therefore reported withConversation: 0 and withRequest: 0 and dropped the structured missing-transcript count. Please either populate the canonical fields here or extend the bounded facts schema and tests so unknown/unmapped evidence is not serialized as confirmed zero.
There was a problem hiding this comment.
Rechecked public facts at 58b93fd against the same native workspace: it reports workspaceSessions: 21, inWindowSessions: 16, outOfWindowSessions: 0, timeUnobservedSessions: 0, relevantSessions: 16, and unreadable: 0, although 5 matching session directories still have no events.jsonl. facts always supplies an until, so requestedWindow is true; relevant narrows to inWindowProbes, while timeUnobserved counts only probes that already have a transcript and outOfWindowSessions subtracts all time-unobserved probes. The 5 missing transcripts therefore remain unaccounted for in every canonical counter. Please keep them visible as time-unobserved/unreadable evidence (or extend the bounded schema) and cover the actual public facts result, not only the internal factsSourceCoverage object.
Applies the four review findings on PR QoderAI#22. The agent-customize CLI parsed --copilot-home but never forwarded it to the collector, so every CLI run inventoried the real ~/.copilot instead of the requested path. That broke the CLI half of CHS-AC-6 while the collector API stayed correct. scripts/agent-customize/cli.mjs now passes copilotHome through, and a CLI regression test asserts the inventory resolves under the fixture home and that no evidence path points at the real user home. The Copilot platform module dropped the outputTokens that assistant messages do record. isModelRequestEvent excludes assistant-typed events, so usage cannot ride the message itself: each assistant message that reports usage now also emits a companion model.response.completed carrying outputTokens, messageId as responseId, and requestId. Input and cache tokens stay absent rather than zero. 10,462 of 11,313 assistant messages across 491 local sessions carry a non-zero outputTokens, so the coverage is genuinely partial and the warning is renamed copilot-per-response-usage-partial. permission.requested and permission.completed now normalize into the shared control.permission lifecycle, retaining only requestId, the request kind, and the result decision; intents, paths, and commands are never retained. The decision rides the result event alone, because permissionObservation counts any event carrying a decision and Copilot emits a request even when policy auto-approves, so decorating both would double-count and label all 1,177 requests as prompted friction. toolInvocationId is deliberately left unset: dedupeEvents keys on it, and 32 toolCallIds repeat across separate permission requests, which would have silently dropped legitimate events. Source coverage now emits the canonical session-core-facts fields that safeSourceCoverage whitelists, keeping the Copilot transcript counters beside them so analysisWarnings keeps working. Two latent bugs surfaced here: coverage read probe fields off session descriptors that never carried them, so withRequest was always zero, and the relevant-set ladder narrowed even without a requested window, hiding transcript-less sessions instead of reporting them as unreadable. CHS-AC-5 and CHS-AC-6 in docs/specs/2026-07-29-copilot-host-support.md now describe partial per-response usage and the payload-free permission lifecycle, and Session Diagnostics documents the two added boundaries. Validated with npm test (874/874), npm run pack:verify (PASS, npm 314 / runtime zip 338), and node --test test/doc-link-graph.test.mjs (6/6, no mermaid drift). Co-authored-by: Copilot App (Claude Opus 5) <223556219+Copilot@users.noreply.github.com>
|
Thanks for the detailed review @phodal — all four findings are fixed. Summary of what changed and why: 1. Confirmed: 2. Per-response usage was dropped (P1) Confirmed and quantified against 491 local Copilot sessions: 10,462 of 11,313
3. Permission lifecycle (P2)
Two design calls worth flagging, since they read as omissions otherwise:
4. Coverage contract (P2) Source coverage now emits the canonical Fixing this surfaced two latent bugs that were hiding real signal:
Validation
Spec updated in the same commit: CHS-AC-5 and CHS-AC-6 in All of the above is in |
Document an evidence-first workflow for adding Coding Agent hosts, drawing bounded lessons from PR QoderAI#6 and PR QoderAI#22. Link the canonical guide from contributor entry points and publish concise English and zh-Hans site routes. The change follows docs/specs/2026-07-30-new-coding-agent-contribution-guide.md and was validated with the 7/7 documentation checks, 867/867 full tests, the two-locale Docusaurus build, package verification, and git diff checks. Co-authored-by: Codex (GPT 5.6 Sol) <codex@openai.com>
Summary
Makes GitHub Copilot a first-class, evidence-safe Better Harness host alongside Qoder, Codex, Claude Code, Cursor, and Qwen Code.
Copilot already loads the canonical
better-harnessSkill today, but only by accident: it resolves plugin manifests in the order.plugin/plugin.json→plugin.json→.github/plugin/plugin.json→.claude-plugin/plugin.json, so it falls through to the Claude Code shell. That fallback is undocumented, gives Copilot no shell of its own, and makes load order depend on which other host shells happen to exist.The larger gap was evidence: every provider enum rejected
copilot, so the workflow could not inventory Copilot's configured assets or read Copilot session transcripts. A Copilot user could run the Skill but could not get a Copilot-scoped Harness report.Spec:
docs/specs/2026-07-29-copilot-host-support.mdWhat's in it
Native shell —
.github/plugin/plugin.json+.github/plugin/marketplace.json, version-pinned topackage.json(0.3.0) like every other host manifest. Copilot now resolves its own shell instead of borrowing Claude Code's.Asset provider —
scripts/agent-customize/providers/copilot.mjs(new, 426 lines). Adds--provider copilot/--copilot-home, inventorying plugin, user (~/.copilot), and project (.github) scopes with the same scope separation as existing providers.Session platform —
scripts/session-analysis/platforms/copilot.mjs(new, 571 lines). Adds--platform copilot, normalizing~/.copilot/session-state/<id>/events.jsonl+workspace.yaml. Copilot's transcript is the richest of any supported host — it records hook, subagent, plan, compaction, and permission lifecycle events directly. Unrecognized event types degrade tometadata.<rawType>rather than being dropped, so future Copilot events never silently vanish.Evidence honesty — Copilot facts report
usageFieldsObserved: falsebecause the transcript records no per-response token usage. The adapter reports partial coverage rather than inventing it, and rejects foreign-workspace sessions.Provider enum registration —
copilotis now accepted across evidence-bundle contract/CLI, report-quality, task-loop, agent-lint, thebetter-harness-cliregistry, coding-agent-practices inventory/baseline/integrity, andpack:verify.Documentation routing fixes — several agent-facing routing docs enumerated providers as
<cursor|qoder|codex|claude>, silently omitting both Qwen and Copilot. An agent reading those docs could never discover--provider copilot(orqwen). Corrected in the agent-customize inventory CLI flags, the global-assets scope + home list, the routing index, the ADR host-adapter comment, and the session-evidence--providerenum.Commits
93abeecfeat:ffd6ab6feat:8f362eadocs:Total: 51 files, +1850/−104.
Each slice was verified standing alone by materializing the index read-only and running the suite against it, so the series is bisectable — no commit in the middle is red.
Verification
npm test— 872 / 872 pass, 0 failnpm run pack:verify— PASS (npm 314 entries, runtime zip 338)node scripts/doc-link-graph/cli.mjs skills/better-harness— 34 files / 50 links, zero driftharness evidence-bundle --platform copilotreturns all three lanes asavailable;--copilot-homeoverride routes correctlyAcceptance criteria
All nine confirmed: CHS-AC-1 (native shell) · AC-2 (version alignment) · AC-3 (configured assets) · AC-4 (session evidence) · AC-5 (evidence boundaries) · AC-6 (bundle propagation) · AC-7 (host routing) · AC-8 (provider behavior) · AC-9 (documentation integrity).
Non-goals
No changes to other hosts' adapters, no new report formats, no changes to coverage semantics. Copilot is added alongside existing hosts, not in place of any.
Open questions for reviewers
plugin.jsonschema — does Copilot publish a versioned JSON schema the manifest test should validate against? Currently the test asserts structure by hand. (tracked as[NEEDS CLARIFICATION]in the spec)~/.copilot/session-statestability — is the layout stable enough to promote from observed to documented in the host matrix? (tracked as[NEEDS CLARIFICATION]in the spec)copilot plugin marketplace add QoderAI/better-harness. I could not verify theowner/reposhorthand against Copilot CLI's real marketplace-add grammar. Please confirm before merge.Reviewer notes
.github/plugin/location is not arbitrary — it is the third entry in Copilot's manifest resolution order and the first that doesn't collide with another host's shell. See the resolution order inscripts/agent-customize/providers/copilot.mjs.test/scripts-refactor-contract.test.mjschanges only two SHA-256 constants. That is a lockstep artifact of the CLI help text gainingcopilot, not new test logic — which is why it ships in thefeatcommit rather than thetestone.test/agent-customize-architecture.test.mjs,better-harness-skill,coding-agent-platform-notes, andstyle-templatesassert on docs prose, so they ship in thedocscommit.Reviewed with
.agents/skills/change-traceability-review/SKILL.md(Review Readiness Check mode).